Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sabrina/ssl errors #2511

Merged
merged 31 commits into from
Apr 12, 2024
Merged

Sabrina/ssl errors #2511

merged 31 commits into from
Apr 12, 2024

Conversation

SabrinaTardio
Copy link
Collaborator

@SabrinaTardio SabrinaTardio commented Mar 27, 2024

Task/Issue URL: https://app.asana.com/0/0/1206880509171835/f
Tech Design URL: https://app.asana.com/0/0/1206862686877685/f
CC:

Description: In case of SSL error will give the user the option of bypass the error and visit the site anyway. See Figma → and https://app.asana.com/0/72649045549333/1206755044584061

Steps to test this PR:
For initial test please check out shane/ssl-updates branch of CSS and move the CSS folder on xcode

  1. Visit any standard website and check shield icon and privacy dashboard are as expected
  2. Go to badssl.com and test the various type of bad ssl
  3. For each one check the error shown is as expected also when clicking advanced...
  4. For each click on leave website and check it takes you back to previous page
  5. For each click on accept risk and visit site and check the website is loaded
  6. Check the shield icon is dotted for sites with bad ssl
  7. Privacy dash board will temporarily look ok until changes are made. https://app.asana.com/0/72649045549333/1206852519463852
  8. Check that the once a bad ssl website is loaded it keeps being loaded until the app is restarted (in that case will show the error again.
  9. Check that trusting one site doesn’t automatically trust others.
  10. Smoke test using different and save tab and check shield icon is always as expected.

Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

Copy link
Collaborator

@ayoy ayoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SabrinaTardio the code looks and works great! 👏 Not approving because it's a draft, but otherwise it's all good.

I've left a few suggestions based on my testing and our chatting. My additional, design-related suggestion would be to make the font bigger in the error pages.

Great job! 🚀

DuckDuckGo/Tab/Model/Tab.swift Outdated Show resolved Hide resolved
DuckDuckGo/Tab/Model/Tab.swift Outdated Show resolved Hide resolved
DuckDuckGo/Tab/TabExtensions/ErrorPageTabExtension.swift Outdated Show resolved Hide resolved
DuckDuckGo/Tab/UserScripts/UserScripts.swift Show resolved Hide resolved
IntegrationTests/Tab/AddressBarTests.swift Show resolved Hide resolved
DuckDuckGo/ErrorPage/ErrorPageHTMLTemplate.swift Outdated Show resolved Hide resolved
@SabrinaTardio SabrinaTardio marked this pull request as ready for review April 6, 2024 18:16
Copy link
Collaborator

@ayoy ayoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job @SabrinaTardio !

Copy link
Contributor

github-actions bot commented Apr 12, 2024

Warnings
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS against 590e8fd

@SabrinaTardio SabrinaTardio merged commit fcf8390 into main Apr 12, 2024
17 checks passed
@SabrinaTardio SabrinaTardio deleted the sabrina/ssl-errors branch April 12, 2024 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants